-
Notifications
You must be signed in to change notification settings - Fork 968
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature branch blob proof #3985
base: feature/api-breaks
Are you sure you want to change the base?
Conversation
|
…3610) Co-authored-by: rene <[email protected]>
Closes #3629 Co-authored-by: Hlib Kanunnikov <[email protected]>
f9a3187
to
29dcf79
Compare
|
29dcf79
to
664255c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 🎉
Great working fixing all the conflicts 🙏 Thanks a lot for helping with this
@@ -6,54 +6,162 @@ import ( | |||
"errors" | |||
"fmt" | |||
|
|||
"github.com/tendermint/tendermint/crypto/merkle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it should be "github.com/celestiaorg/go-square/merkle"
😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for other files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I see that in go.mod
github.com/celestiaorg/go-square/merkle
got removed. Also, in go-square/v2
there is no merkle
package anymore. So, I might be wrong about bad import.
return ErrInvalidProof | ||
// Verify takes a data root and verifies if the | ||
// provided proof's subtree roots were committed to the given data root. | ||
func (p *Proof) Verify(dataRoot []byte) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a bool
return param at all? Looks like it's only true
when error is nil
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. Double-checked and it seems we don't have a case when we have to return false, nil
or true,error
} | ||
|
||
func (com Commitment) String() string { | ||
return string(com) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be hex or base64 ?
btw, given that we have this PR, can we close this one #3796? |
Yeah I think so |
Merge Blob Proof feature branch to api/breaks